Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

uplift to stable2407 #1372

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

uplift to stable2407 #1372

wants to merge 7 commits into from
+1,865 −2,058

Conversation

vedhavyas
Copy link
Contributor

@vedhavyas vedhavyas commented Oct 16, 2024 •
edited
Loading

Pull Request Summary
PR uplifts to stable 2407.
Commit one uplifts the project
Commit two bumps the versions

Check list

  • added or updated unit tests
  • updated Astar official documentation
  • added OnRuntimeUpgrade hook for precompile revert code registration
  • added benchmarks & weights for any modified runtime logics.

@vedhavyas vedhavyas added astar Related to Astar runtime This PR/Issue is related to the topic “runtime”. client This PR/Issue is related to the topic “client”. labels Oct 16, 2024
Dinonard
Dinonard reviewed Oct 16, 2024
View reviewed changes
Copy link
Member

@Dinonard Dinonard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the treasury changes, we also need to update the docs and inform the Subsquare team to update the UI logic.

bin/collator/Cargo.toml Outdated Show resolved Hide resolved
pallets/xc-asset-config/src/tests.rs Outdated Show resolved Hide resolved
runtime/astar/src/lib.rs Outdated Show resolved Hide resolved
runtime/local/src/lib.rs Outdated Show resolved Hide resolved
runtime/shibuya/src/lib.rs Outdated Show resolved Hide resolved
vedhavyas
vedhavyas commented Oct 17, 2024
View reviewed changes
Copy link
Contributor Author

@vedhavyas vedhavyas left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New treasury changes requires the origin to return the Max amount to spend at a time and I have set the Max to Max Balance.

As for the Payout timeout, I have set 3 days for shibuya and 30 minutes for Local. After this period, the spends are expired

For the treasury changes, we also need to update the docs and inform the Subsquare team to update the UI logic.

@Dinonard How do I start doing that ?

runtime/shibuya/src/lib.rs Outdated Show resolved Hide resolved
ipapandinas
ipapandinas previously approved these changes Oct 17, 2024
View reviewed changes
Copy link
Contributor

@ipapandinas ipapandinas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

bin/collator/src/parachain/service.rs Show resolved Hide resolved
ipapandinas
ipapandinas reviewed Oct 17, 2024
View reviewed changes
Copy link
Contributor

@ipapandinas ipapandinas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's missing xcmp-queue v5 migrations as Ermal noticed

@vedhavyas
Copy link
Contributor Author

It's missing xcmp-queue v5 migrations as Ermal noticed

yes I'min the process of adding those :)

@vedhavyas
Copy link
Contributor Author

@ipapandinas Due to this recent patch to stable2407 paritytech/polkadot-sdk#5913
I had to adjust xcm dry tests. FYI since you seems to have written them

@github-actions GitHub Actions
Copy link

Code Coverage

Package Line Rate Branch Rate Health
pallets/astar-xcm-benchmarks/src/fungible 100% 0% ✔
chain-extensions/types/assets/src 0% 0% ❌
pallets/inflation/src 89% 0% ✔
pallets/dapp-staking/src 83% 0% ✔
pallets/dapp-staking/rpc/runtime-api/src 0% 0% ❌
pallets/dapp-staking/src/benchmarking 98% 0% ✔
pallets/price-aggregator/src 82% 0% ✔
precompiles/dapp-staking/src/test 0% 0% ❌
chain-extensions/unified-accounts/src 0% 0% ❌
pallets/vesting-mbm/src 89% 0% ✔
precompiles/dispatch-lockdrop/src 86% 0% ✔
chain-extensions/types/unified-accounts/src 0% 0% ❌
precompiles/dapp-staking/src 90% 0% ✔
pallets/astar-xcm-benchmarks/src 86% 0% ✔
pallets/unified-accounts/src 86% 0% ✔
pallets/ethereum-checked/src 74% 0% ➖
precompiles/unified-accounts/src 100% 0% ✔
chain-extensions/pallet-assets/src 56% 0% ➖
precompiles/substrate-ecdsa/src 74% 0% ➖
pallets/collective-proxy/src 86% 0% ✔
pallets/dynamic-evm-base-fee/src 89% 0% ✔
precompiles/sr25519/src 64% 0% ➖
pallets/collator-selection/src 92% 0% ✔
pallets/astar-xcm-benchmarks/src/generic 100% 0% ✔
primitives/src/xcm 65% 0% ➖
pallets/static-price-provider/src 85% 0% ✔
pallets/xc-asset-config/src 50% 0% ❌
precompiles/xcm/src 71% 0% ➖
precompiles/assets-erc20/src 78% 0% ➖
pallets/dapp-staking/src/test 0% 0% ❌
primitives/src 57% 0% ➖
Summary 79% (3730 / 4738) 0% (0 / 0) ➖

Minimum allowed line rate is 50%

ipapandinas
ipapandinas approved these changes Oct 21, 2024
View reviewed changes
Copy link
Contributor

@ipapandinas ipapandinas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch on the XCM dry-run patch! those tests are purely integration tests to ensure that the runtime API is correctly implemented

now that you've reversed the treasury updates, the changes are lighter, so I approve the uplift ✅

Good job! 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ipapandinas ipapandinas ipapandinas approved these changes

@Dinonard Dinonard Awaiting requested review from Dinonard

@ermalkaleci ermalkaleci Awaiting requested review from ermalkaleci

Assignees
No one assigned
Labels
astar Related to Astar client This PR/Issue is related to the topic “client”. runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vedhavyas @Dinonard @ermalkaleci @ipapandinas